Skip to content

Request #41187 DateTime::setDefaultFormat and __toString()#103

Closed
Powerhamster wants to merge 1 commit into
php:masterfrom
Powerhamster:issue-41187
Closed

Request #41187 DateTime::setDefaultFormat and __toString()#103
Powerhamster wants to merge 1 commit into
php:masterfrom
Powerhamster:issue-41187

Conversation

@Powerhamster

Copy link
Copy Markdown
  • added static protected DateTime::$defaultFormat
  • added bool DateTime::setDefaultFormat($format)
  • added string DateTime::getDefaultFormat()
  • added DateTime::__toString()
  • DateTime::$defaultFormat supports LSB

This should fix some issues with Datetime for libraries like e.g. doctrine 2 which can not implement a special handling for Datetime because of performance issues.

    - added static protected DateTime::$defaultFormat
    - added bool DateTime::setDefaultFormat($format)
    - added string DateTime::getDefaultFormat()
    - added DateTime::__toString()
    - DateTime::$defaultFormat supports LSB
@Powerhamster

Copy link
Copy Markdown
Author

@dsp If you have meant DateTime_verify.phpt. Yes, I did a copy of the output of the Testscript.
%d is replaced intentionally, because of order of methods is fixed and should be the same.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a good idea - what it is testing? Fixed order of values in the hash? Why would we want to test for that? You yourself can see that this leads to a lot of changes each time method is added, why do that? I think %d's should be left alone.

@smalyshev

Copy link
Copy Markdown
Contributor

I think RFC for such thing is a good idea. Also, looks like this API means that __toString() must work the same for all the dates in the application. Why such requirement is necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants